-
Notifications
You must be signed in to change notification settings - Fork 34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Street geo #516
Street geo #516
Conversation
✅ Deploy Preview for 3dstreet-core-builds ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
I have changed this example for testing:
The lat/long change still needs to be tested |
src/components/street-geo.js
Outdated
// <mapName> : <function that creates and return map element> | ||
'mapbox2d': this.createMapbox2dElement.bind(this), | ||
'google3d': this.createGoogle3dElement.bind(this) | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mapTypes
could be defined in init
not update
I renamed the map creation functions. Now the function has a name: MapType + "Create". Similar to the update functions: MapType + "Update". |
Elevation is the requested elevation of the scene's origin 0 0 0. In other words, if a user has lat 30, long 30, elevation 30, then I assume the elevation is 30 meters above sea level. But google 3d tiles takes height which is a different value. Here is a sample of how I converted elevation into height in a glitch prototype project:
|
calculate elevation with constant height value for now
src/components/street-geo.js
Outdated
|
||
const mapbox2dElement = document.createElement('a-entity'); | ||
mapbox2dElement.setAttribute('data-layer-name', 'Mapbox Satellite Streets'); | ||
mapbox2dElement.setAttribute('geometry', 'primitive: plane; width: 512; height: 512;'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have a mix of spaces and tabs in the file, be sure to replace all tabs by spaces.
src/components/street-geo.js
Outdated
}); | ||
|
||
}, | ||
removeChildMaps: function () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't seem to be used.
Next:
|
added documentation and test based on |
I also added a |
expect(mapbox2dElement).to.exist; | ||
expect(google3dElement).to.exist; | ||
done(); | ||
}, 1000); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you really need to wait 1s? We don't have much tests so that may be fine but if we can avoid that it would be better. You probably just need two setTimeout to be sure the street-geo is initialized and the child entity with components are created and initialized.
setTimeout(() => {
setTimeout(() => {
const mapbox2dElement = el.querySelector('[data-layer-name="Mapbox Satellite Streets"]');
const google3dElement = el.querySelector('[data-layer-name="Google 3D Tiles"]');
expect(mapbox2dElement).to.exist;
expect(google3dElement).to.exist;
done();
});
});
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the tip!
For tests, the main convention I saw in several projects is to mirror the directory structure and end the test filename with .spec.js or .test.js. The aframe repository for example uses .test.js convention.
|
Those new tests should be able to be run with mocha and jsdom, no need to spawn a browser. I see existing tests were using jsdom-global already. |
Hi, existing tests in 3DStreet do not use AFRAME. I had difficulty to run AFRAME in Node.js environment. And I thought that running AFRAME in a browser environment with the |
I tried using only
that I haven't found a way to fix. I see these options so far:
Another option is to further try to fix the errors in point 1, that is, change the three.js code referenced by Aframe. I already had this experience a year ago when I adapted Three.js to work on the server side. But I think it would be easier to do 2 or 3 |
The aframe doc is not be up to date, using |
So yeah for now executing the tests manually by accessing |
FYI I made this PR aframevr/aframe#5522 |
No description provided.